MDEV-38632 ALTER EVENT doesn't re-run one-time AT event after first execution#5054
MDEV-38632 ALTER EVENT doesn't re-run one-time AT event after first execution#5054keeper wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request addresses MDEV-38632, ensuring that one-time events with ON COMPLETION PRESERVE are correctly rescheduled and re-enabled when modified via ALTER EVENT. The changes update the event queue logic to check if a one-time event is pending based on its execution timestamp and modify the database repository to clear execution history and restore the ENABLED status upon schedule changes. Review feedback highlights the need for safer error handling when retrieving timestamps in sql/event_db_repository.cc to avoid potential undefined behavior from uninitialized memory if get_date fails.
gkodinov
left a comment
There was a problem hiding this comment.
Thank you for your contribution. Please fix the tests failing on debug builds with a dbug assert as follows:
Thread 1 (Thread 0x7f696e2c1640 (LWP 281786)):
#0 0x00007f69751019fc in pthread_kill () from /lib/x86_64-linux-gnu/libc.so.6
No symbol table info available.
#1 0x0000557c6275b316 in my_write_core (sig=6) at /home/buildbot/amd64-ubuntu-2204-debug/build/mysys/stacktrace.c:424
No locals.
#2 0x0000557c61ccebdc in handle_fatal_signal (sig=6) at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/signal_handler.cc:298
curr_time = 1778188402
tm = {tm_sec = 22, tm_min = 13, tm_hour = 21, tm_mday = 7, tm_mon = 4, tm_year = 126, tm_wday = 4, tm_yday = 126, tm_isdst = 0, tm_gmtoff = 0, tm_zone = 0x7f6975244afb "UTC"}
thd = 0x7f6948004e68
#3 <signal handler called>
No symbol table info available.
#4 0x00007f69751019fc in pthread_kill () from /lib/x86_64-linux-gnu/libc.so.6
No symbol table info available.
#5 0x00007f69750ad476 in raise () from /lib/x86_64-linux-gnu/libc.so.6
No symbol table info available.
#6 0x00007f69750937f3 in abort () from /lib/x86_64-linux-gnu/libc.so.6
No symbol table info available.
#7 0x00007f697509371b in ?? () from /lib/x86_64-linux-gnu/libc.so.6
No symbol table info available.
#8 0x00007f69750a4e96 in __assert_fail () from /lib/x86_64-linux-gnu/libc.so.6
No symbol table info available.
#9 0x0000557c61c85ed8 in TIME_from_longlong_datetime_packed (ltime=0x7f696e2bf830, tmp=-9223372036854775808) at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/compat56.cc:270
ymd = 140090158264344
hms = -549755813888
ymdhms = 1848375056
ym = 140090796668720
__PRETTY_FUNCTION__ = "void TIME_from_longlong_datetime_packed(MYSQL_TIME*, longlong)"
#10 0x0000557c61ca2a95 in Field_datetimef::get_TIME (this=0x7f69481b7068, ltime=0x7f696e2bf830, pos=0x7f69481eb018 "", fuzzydate={m_mode = (date_mode_t::FRAC_TRUNCATE | date_mode_t::NO_ZERO_DATE)}) at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/field.cc:7387
__PRETTY_FUNCTION__ = "virtual bool Field_datetimef::get_TIME(MYSQL_TIME*, const uchar*, date_mode_t) const"
tmp = -9223372036854775808
#11 0x0000557c61cb84ce in Field_datetimef::get_date (this=0x7f69481b7068, ltime=0x7f696e2bf830, fuzzydate={m_mode = (date_mode_t::FRAC_TRUNCATE | date_mode_t::NO_ZERO_DATE)}) at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/field.h:4122
No locals.
#12 0x0000557c61f3c213 in mysql_event_fill_row (thd=0x7f6948004e68, table=0x7f69480f1a68, et=0x7f694801b1b8, sp=0x0, sql_mode=0, is_update=1 '\001') at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/event_db_repository.cc:342
old_execute_at = {year = 0, month = 0, day = 0, hour = 0, minute = 0, second = 0, second_part = 0, neg = 0 '\000', time_type = MYSQL_TIMESTAMP_DATE}
schedule_changed = false
tz_name = 0x557c638c3980 <tz_SYSTEM_name>
time = {year = 1970, month = 1, day = 2, hour = 0, minute = 0, second = 0, second_part = 0, neg = 0 '\000', time_type = MYSQL_TIMESTAMP_DATETIME}
scs = 0x557c6388be60 <my_charset_utf8mb3_general1400_as_ci>
f_num = ET_FIELD_NAME
fields = 0x7f69481ebc68
rs = 0
__PRETTY_FUNCTION__ = "bool mysql_event_fill_row(THD*, TABLE*, Event_parse_data*, sp_head*, sql_mode_t, my_bool)"
#13 0x0000557c61f3d545 in Event_db_repository::update_event (this=0x557c6e07f4a0, thd=0x7f6948004e68, parse_data=0x7f694801b1b8, new_dbname=0x0, new_name=0x0) at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/event_db_repository.cc:891
scs = 0x557c6388be60 <my_charset_utf8mb3_general1400_as_ci>
table = 0x7f69480f1a68
sp = 0x0
saved_mode = 0
mdl_savepoint = {m_stmt_ticket = 0x7f69481418f0, m_trans_ticket = 0x7f694825f9d0}
ret = 1
__PRETTY_FUNCTION__ = "bool Event_db_repository::update_event(THD*, Event_parse_data*, LEX_CSTRING*, LEX_CSTRING*)"
#14 0x0000557c619f9917 in Events::update_event (thd=0x7f6948004e68, parse_data=0x7f694801b1b8, new_dbname=0x0, new_name=0x0) at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/events.cc:530
ret = 1665952136
save_binlog_format = BINLOG_FORMAT_STMT
new_element = 0x557c61a55989 <MDL_context::release_transactional_locks(THD*)+209>
__PRETTY_FUNCTION__ = "static bool Events::update_event(THD*, Event_parse_data*, LEX_CSTRING*, LEX_CSTRING*)"
#15 0x0000557c6181f97f in mysql_execute_command (thd=0x7f6948004e68, is_called_from_prepared_stmt=false) at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/sql_parse.cc:5191
res = 0
lex = 0x7f69480094a0
select_lex = 0x7f6948009e00
first_table = 0x0
all_tables = 0x0
unit = 0x7f6948009580
__PRETTY_FUNCTION__ = "int mysql_execute_command(THD*, bool)"
ots = {ctx = 0x7f6948009200, traceable = false}
orig_binlog_format = BINLOG_FORMAT_MIXED
orig_current_stmt_binlog_format = BINLOG_FORMAT_STMT
#16 0x0000557c6182821c in mysql_parse (thd=0x7f6948004e68, rawbuf=0x7f694801b030 "ALTER EVENT event_starts_test ON SCHEDULE AT '1970-01-02 00:00:00' ON COMPLETION PRESERVE DISABLE", length=97, parser_state=0x7f696e2c01d0) at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/sql_parse.cc:7941
found_semicolon = 0x0
error = 32617
lex = 0x7f69480094a0
err = false
__PRETTY_FUNCTION__ = "void mysql_parse(THD*, char*, uint, Parser_state*)"
#17 0x0000557c61814e46 in dispatch_command (command=COM_QUERY, thd=0x7f6948004e68, packet=0x7f694808f299 "ALTER EVENT event_starts_test ON SCHEDULE AT '1970-01-02 00:00:00' ON COMPLETION PRESERVE DISABLE", packet_length=97, blocking=true) at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/sql_parse.cc:1898
packet_end = 0x7f694801b091 ""
parser_state = {m_lip = {lookahead_token = -1, lookahead_yylval = 0x0, m_thd = 0x7f6948004e68, m_ptr = 0x7f694801b092 "\v", m_tok_start = 0x7f694801b092 "\v", m_tok_end = 0x7f694801b092 "\v", m_end_of_query = 0x7f694801b091 "", m_tok_start_prev = 0x7f694801b091 "", m_buf = 0x7f694801b030 "ALTER EVENT event_starts_test ON SCHEDULE AT '1970-01-02 00:00:00' ON COMPLETION PRESERVE DISABLE", m_buf_length = 97, m_echo = true, m_echo_saved = true, m_cpp_buf = 0x7f694801b0f0 "ALTER EVENT event_starts_test ON SCHEDULE AT '1970-01-02 00:00:00' ON COMPLETION PRESERVE DISABLE", m_cpp_ptr = 0x7f694801b151 "", m_cpp_tok_start = 0x7f694801b151 "", m_cpp_tok_start_prev = 0x7f694801b151 "", m_cpp_tok_end = 0x7f694801b151 "", m_body_utf8 = 0x0, m_body_utf8_ptr = 0x7f69481096f1 '\217' <repeats 200 times>..., m_cpp_utf8_processed_ptr = 0x0, next_state = MY_LEX_END, found_semicolon = 0x0, ignore_space = false, stmt_prepare_mode = false, multi_statements = true, hint_comment = false, yylineno = 1, m_digest = 0x0, in_comment = NO_COMMENT, in_comment_saved = (DISCARD_COMMENT | unknown: 0x5a5a5a58), m_cpp_text_start = 0x7f694801b11e "1970-01-02 00:00:00' ON COMPLETION PRESERVE DISABLE", m_cpp_text_end = 0x7f694801b131 "' ON COMPLETION PRESERVE DISABLE", m_underscore_cs = 0x0}, m_yacc = {yacc_yyss = 0x0, yacc_yyvs = 0x0, m_set_signal_info = {m_item = {0x0 <repeats 13 times>}}, m_lock_type = TL_READ_DEFAULT, m_mdl_type = MDL_SHARED_READ}, m_digest_psi = 0x7f6948008f88}
net = 0x7f6948005270
error = false
do_end_of_statement = true
log_slow_done = false
drop_more_results = false
__PRETTY_FUNCTION__ = "dispatch_command_return dispatch_command(enum_server_command, THD*, char*, uint, bool)"
__FUNCTION__ = "dispatch_command"
res = <optimized out>
#18 0x0000557c6181398c in do_command (thd=0x7f6948004e68, blocking=true) at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/sql_parse.cc:1432
return_value = DISPATCH_COMMAND_SUCCESS
packet = 0x7f694808f298 "\003ALTER EVENT event_starts_test ON SCHEDULE AT '1970-01-02 00:00:00' ON COMPLETION PRESERVE DISABLE"
packet_length = 98
net = 0x7f6948005270
command = COM_QUERY
__PRETTY_FUNCTION__ = "dispatch_command_return do_command(THD*, bool)"
__FUNCTION__ = "do_command"
#19 0x0000557c61a3d0d1 in do_handle_one_connection (connect=0x557c6e17f6a8, put_in_cache=true) at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/sql_connect.cc:1503
create_user = true
thr_create_utime = 18796833249641
thd = 0x7f6948004e68
#20 0x0000557c61a3ce77 in handle_one_connection (arg=0x557c6e0aed58) at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/sql_connect.cc:1415
connect = 0x557c6e0aed58
#21 0x0000557c6205898d in pfs_spawn_thread (arg=0x557c6e0aee38) at /home/buildbot/amd64-ubuntu-2204-debug/build/storage/perfschema/pfs.cc:2198
typed_arg = 0x557c6e0aee38
user_arg = 0x557c6e0aed58
user_start_routine = 0x557c61a3ce09 <handle_one_connection(void*)>
pfs = 0x7f69746fe940
klass = 0x557c6d898940
e91440e to
cb709ba
Compare
|
Hi @gkodinov , thanks for review! I've addressed the review from Gemini, by:
and that solves the assertion failure in debug test. |
gkodinov
left a comment
There was a problem hiding this comment.
LGTM. So ideas on improving the test are outlined below.
I do not quite subscribe to the idea of the fix (implement the logic inside the table maintenance run instead of in the runtime data run) but since this is a preliminary review I'd leave that to the final reviewer.
Please stay tuned for the final review.
gkodinov
left a comment
There was a problem hiding this comment.
Hmm, one thing I've forgot: this is a bug fix. Please rebase it to 10.11.
cb709ba to
556915e
Compare
b145e93 to
556915e
Compare
…xecution A one-time event (ON SCHEDULE AT ... ON COMPLETION PRESERVE) could not be rescheduled via ALTER EVENT after its first execution. The event would silently fail to fire again. Root cause: After a one-time AT event executes, compute_next_execution_time() sets status=DISABLED and persists both the DISABLED status and last_executed to mysql.event. When the user reschedules with ALTER EVENT ... ENABLE, the status is updated but last_executed remains stale. On server restart, load_events_from_db() calls compute_next_execution_time() which sees last_executed != 0 and unconditionally disables the event again, even though execute_at was changed to a future time. Fix: - event_db_repository.cc (mysql_event_fill_row): When ALTER EVENT changes execute_at to a new value, clear last_executed in mysql.event. This ensures the event is treated as not-yet-executed for the new schedule. - event_data_objects.cc (compute_next_execution_time): For one-time events, only disable if last_executed >= execute_at, meaning the event was actually executed for the current schedule. If execute_at is after last_executed (rescheduled), treat it as pending. This serves as a safety net for server restarts where last_executed may persist from an older schedule. All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.
556915e to
f441b87
Compare
|
To summarize my latest change:
|
gkodinov
left a comment
There was a problem hiding this comment.
LGTM. Thanks for your continued work on this.
Please stand by for the final review.
There was a problem hiding this comment.
Pull request overview
Fixes MDEV-38632 where one-time ON SCHEDULE AT ... ON COMPLETION PRESERVE events can fail to fire after being rescheduled via ALTER EVENT, especially across reload/restart scenarios where last_executed persists and causes the scheduler to treat the event as already executed.
Changes:
- Clear
mysql.event.last_executedonALTER EVENTwhenexecute_atchanges (to avoid stalelast_executeddisabling the rescheduled event on reload). - Adjust one-time event execution logic to only auto-disable when
last_executed >= execute_at(so a rescheduledexecute_atafterlast_executedremains pending). - Add an MTR regression test and expected result for the reschedule/restart scenarios.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
sql/event_db_repository.cc |
Clears last_executed when a one-time event’s execute_at changes during ALTER. |
sql/event_data_objects.cc |
Refines one-time event disable check to require last_executed >= execute_at. |
mysql-test/main/mdev38632_alter_onetime_event.test |
Adds regression test coverage for rescheduling one-time events and restart reload behavior. |
mysql-test/main/mdev38632_alter_onetime_event.result |
Adds expected output for the new MTR test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Please address copilot changes otherwise it looks OK |
@sanja-byelkin The copilot got fooled by the stale PR explanation that was not edited to match the extent of the actual fix proposed. I have replied to the copilot comments now and updated the PR description. |
sanja-byelkin
left a comment
There was a problem hiding this comment.
The idea is good but have to be fixed.
| @@ -0,0 +1,97 @@ | |||
| # | |||
| # MDEV-38632: ALTER EVENT doesn't run a one-time (AT) event after its first | |||
There was a problem hiding this comment.
--echo missed to be in the output
| # MDEV-38632: ALTER EVENT doesn't run a one-time (AT) event after its first | ||
| # execution when ON COMPLETION PRESERVE is used. | ||
| # | ||
| # After an AT event with ON COMPLETION PRESERVE executes, the scheduler sets |
There was a problem hiding this comment.
this is not needed in the test file
| DO INSERT INTO test.event_log(id) VALUES (NULL); | ||
|
|
||
| --echo # Status should remain DISABLED (no schedule change) | ||
| SELECT status FROM mysql.event WHERE db = 'test' AND name = 'mdev38632'; |
There was a problem hiding this comment.
end version marker is missed
--echo # End of 10.11 tests
| --source include/running_event_scheduler.inc | ||
|
|
||
| --echo # The event should fire after server restart. | ||
| --let $wait_timeout = 10 |
There was a problem hiding this comment.
Why 10 sec? what is wrong with standard 30 sec?
A one-time event (ON SCHEDULE AT ... ON COMPLETION PRESERVE) could not
be rescheduled via ALTER EVENT after its first execution. The event
would silently fail to fire again.
Root cause: After a one-time AT event executes,
compute_next_execution_time() sets status=DISABLED and persists both
the DISABLED status and last_executed to mysql.event. When the user
reschedules with ALTER EVENT ... ENABLE, the status is updated but
last_executed remains stale. On server restart, load_events_from_db()
calls compute_next_execution_time() which sees last_executed != 0 and
unconditionally disables the event again, even though execute_at was
changed to a future time.
Fix:
event_db_repository.cc (mysql_event_fill_row): When ALTER EVENT
changes execute_at to a new value, clear last_executed in mysql.event.
This ensures the event is treated as not-yet-executed for the new
schedule.
event_data_objects.cc (compute_next_execution_time): For one-time
events, only disable if last_executed >= execute_at, meaning the event
was actually executed for the current schedule. If execute_at is after
last_executed (rescheduled), treat it as pending. This serves as a
safety net for server restarts where last_executed may persist from an
older schedule.
All new code of the whole pull request, including one or several files
that are either new files or modified ones, are contributed under the
BSD-new license. I am contributing on behalf of my employer Amazon Web
Services, Inc.